Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(jruby): XML::DocumentFragment.dup to another document (v1.17.x) #3373

Merged
merged 1 commit into from
Dec 12, 2024

Conversation

flavorjones
Copy link
Member

Backport of #3372

Back in b92660e (#1834 fixing #1063) I omitted support in JRuby for
the "new_parent_document" argument to `Node#dup` because there was no
performance reason to implement it. So the test was skipped.

However, in 1e7d38a and other commits in #3117 (fixing #316), I
introduced a call to `initialize_copy_with_args` that passes the new
parent document as an argument on both CRuby and JRuby
implementations. Because the test was skipped, I didn't catch that
this broke on JRuby.

In particular this was a problem for Loofah which relies on
decorators, and even more particularly this broke the
`Loofah::TextBehavior` formatting concern for
`Loofah::*::DocumentFragment` objects.

Maybe we should be running downstream tests with JRuby, too? But that
feels like a big investment right now so I'll avoid scarring on the
first cut, and wait to see if it happens again.

(cherry picked from commit dda0be2)
@flavorjones flavorjones added platform/jruby backport Backport of a PR to the current release branch labels Dec 12, 2024
@flavorjones flavorjones added this to the v1.17.x patch releases milestone Dec 12, 2024
@flavorjones flavorjones merged commit ffaa44c into v1.17.x Dec 12, 2024
132 checks passed
@flavorjones flavorjones deleted the flavorjones-jruby-fragment-dup_v1.17.x branch December 12, 2024 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport Backport of a PR to the current release branch platform/jruby
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant